Conversation
Co-authored-by: Sam Bull <aa6bs0@sambull.org>
| def config(self) -> type[BaseModel]: | ||
| configs = {b.module.name: (b.module.default_config | None, None) for b in self.blueprints} | ||
| configs["g"] = (GlobalConfig | None, None) | ||
| return create_model("BlueprintConfig", __config__={"extra": "forbid"}, **configs) # type: ignore[call-overload,no-any-return] |
There was a problem hiding this comment.
Are you sure the # type: ignore is needed?
There was a problem hiding this comment.
We're doing a fair number of dynamic things in this PR, so there are a few things where the static typing in not as good as I'd like..
| main.callback()(create_dynamic_callback()) # type: ignore[no-untyped-call] | ||
|
|
||
|
|
||
| def arghelp( |
| " rerunbridgemodule:", | ||
| " * rerunbridgemodule.frame_id_prefix: str | None (default: foo)", | ||
| " * rerunbridgemodule.frame_id: str | None (default: None)", | ||
| " * rerunbridgemodule.min_interval_sec: float (default: 0.1)", | ||
| " * rerunbridgemodule.entity_prefix: str (default: world)", | ||
| " * rerunbridgemodule.topic_to_entity: collections.abc.Callable[[typing.Any], str] | None (default: None)", | ||
| " * rerunbridgemodule.viewer_mode: typing.Literal['native', 'web', 'connect', 'none'] (default: web)", | ||
| " * rerunbridgemodule.connect_url: str (default: rerun+http://127.0.0.1:9877/proxy)", | ||
| " * rerunbridgemodule.memory_limit: str (default: 25%)", | ||
| f" * rerunbridgemodule.blueprint: collections.abc.Callable[rerun.blueprint.api.Blueprint] | None (default: {_default_blueprint})", |
There was a problem hiding this comment.
I assume these values will change more often. The test could be a bit more robust if it asserted against a modules created for this test only.
There was a problem hiding this comment.
Yeah, can do. My initial thought was to have a test with some real modules to have some realistic coverage.
| "module1": {"arg1": 5} | ||
| } | ||
| config = base_blueprint.config() | ||
| config(**blueprint_args) # raises pydantic.ValidationError if args are incorrect |
There was a problem hiding this comment.
Would be nice to have a test for this.
|
|
||
| try: | ||
| # Not a dependency, just the best way to get config path if available. | ||
| from gi.repository import GLib # type: ignore[import-untyped,import-not-found] |
There was a problem hiding this comment.
Why is this better than using XDG_CONFIG_HOME?
If it is, it might be better to use this in all places where XDG vars are used currently:
dimos/utils/logging_config.py:86: # Running from an installed package - use XDG_STATE_HOME
dimos/utils/logging_config.py:87: xdg_state_home = os.getenv("XDG_STATE_HOME")
dimos/utils/data.py:42: # Use XDG_DATA_HOME if set, otherwise default to ~/.local/share
dimos/utils/data.py:43: xdg_data_home = os.environ.get("XDG_DATA_HOME")
dimos/core/run_registry.py:32: """XDG_STATE_HOME compliant state directory for dimos."""
dimos/core/run_registry.py:33: xdg = os.environ.get("XDG_STATE_HOME")
dimos/perception/experimental/temporal_memory/temporal_memory.py:166: xdg = os.environ.get("XDG_STATE_HOME")
There was a problem hiding this comment.
Oh, didn't realise it was already used elsewhere. I copied this from my previous projects when I spent some time figuring out the best way to get the config dir.
There was a problem hiding this comment.
GLib will do the heavy lifting here and has better support cross-platform. If it's installed, it's going to be the most accurate method. But not worth depending on it.
2 participants
Problem
Users need to be able to pass configuration options for blueprints. This adds basic support for CLI arguments, envvars and config files.
Closes #1082.
Solution
Typer is not compatible with this kind of dynamic configuration, therefore we basically tell it to give us arbitrary strings via
-oand then do all the processing ourselves, along with providing our own help output.How to Test
Should be able to add
--helpto dimos run to see blueprint options and then configure them with-o, env vars and JSON config files. For example:RERUNBRIDGEMODULE__MEMORY_LIMIT="50%" dimos --simulation run unitree-go2 -o voxelgridmapper.voxel_size=1 --config=foo.json